[Geneva] error out for missing deps when building as external component #565
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a build issue in the Fluentd exporter when opentelemetry-cpp is built as an external component. The change prevents incorrect path resolution for the nlohmann_json dependency by requiring it to be pre-installed rather than attempting to download it.
- Adds conditional logic to check if building as main project before attempting to clone nlohmann_json
- Replaces automatic dependency download with a fatal error when nlohmann_json is missing in external builds
| include_directories(${nlohmann_json_SOURCE_DIR}) | ||
| message("nlohmann_json package was not found. Cloning from github") | ||
| else() | ||
| message(FATAL_ERROR "nlohmann_json package was not found which is required for opentelemetry-cpp-fluentd. Please install it") |
There was a problem hiding this comment.
The error message could be more helpful by providing installation guidance. Consider adding information about how to install nlohmann_json (e.g., package manager commands or minimum version requirements).
Suggested change
| message(FATAL_ERROR "nlohmann_json package was not found which is required for opentelemetry-cpp-fluentd. Please install it") | |
| message(FATAL_ERROR "nlohmann_json package was not found, which is required for opentelemetry-cpp-fluentd. Please install it using a package manager. For example:\n" | |
| " - vcpkg: ./vcpkg install nlohmann-json\n" | |
| " - apt (Ubuntu): sudo apt install nlohmann-json-dev\n" | |
| " - Homebrew (macOS): brew install nlohmann-json\n" | |
| "Ensure the package is available and meets the minimum version requirements.") |
There was a problem hiding this comment.
I think the dependence installation instructions are not needed here.
lalitb
approved these changes
Jul 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CMAKE_SOURCE_DIRis used to download nlohmann_json from github when it is not installed. But it is expected to beCMAKE_CURRENT_SOURCE_DIRwhich is different thanCMAKE_SOURCE_DIRwhen building as an external component of opentelemetry-cpp. This PR simplifies this by raising error if nlohmann is not installed before building.